-
Notifications
You must be signed in to change notification settings - Fork 211
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support multiple tsconfig files in expect #973
Conversation
Updated to not use settings, and instead specify tsconfigs in package.json. For sake of dt-mergebot, I'm also going to update things to say that the tsconfigs need to have a specific prefix if not named tsconfig.json. |
packages/dtslint/src/index.ts
Outdated
const tsconfigPath = joinPaths(dirPath, tsconfig); | ||
const tsconfigErrors = checkTsconfig(dirPath, getCompilerOptions(tsconfigPath)); | ||
if (tsconfigErrors.length > 0) { | ||
errors.push("\n\t* " + tsconfigErrors.join("\n\t* ")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it clear from the error text which tsconfig this applies to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, definitely not, will fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modified this to include a filename before each of these groups.
I'm going to hold off on merging this because I want to finish a PR that would work for Node. |
This is all working; I have not done the dt-mergebot change, but since that testing requires an example PR, I'll just merge this, make my node PR ready, and then use that to update dt-mergebot. |
Also, copying my thoughts on this from the discord:
|
To make this work, a package can configure package.json with
These must be named like
tsconfig.*.json
. They must not start with./
or anything and must be present intsX.Y
dirs.This still works less than optimally because ts-eslint needs to be able to match a tsconfig for every file, which means the main
tsconfig
still has to include every file. This means that if you want a tsconfig that excludes a file, you need three tsconfigs in order to still have one with everything.e.g., node has DOM and non-dom; if you want to test both without using expect's
||
, then you need to have two differing test files, but then that means three tsconfigs: one with both, then two with one of each.TODO:
tsconfig.*.json
.